-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Capture Checking of Lazy Vals #24261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Under this scheme, override def to[C1](factory: Factory[A, C1]): C1 = _sorted.to(factory)Because we expect to return a pure (I'm glad that this appears to be the only place in the library that causes trouble). |
6ed8caa to
0b1eb9c
Compare
|
After some more thought, it's actually quite puzzling that the I've now reverted to the previous solution with just |
a1e1c6f to
186df5e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass LGTM
| def methodMember(): String | ||
| lazy val lazyMember: String | ||
|
|
||
| def test(t: T^): Unit = | ||
| // Both require {t} in the capture set | ||
| val m: () ->{t} String = () => t.methodMember() | ||
| val l: () ->{t} String = () => t.lazyMember |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps just use methods with no parameter lists?
trait T:
def methodMember: String| def apply(i: Int): A = _reversed.apply(i) | ||
| def length: Int = len | ||
| def iterator: Iterator[A] = Iterator.empty ++ _reversed.iterator // very lazy | ||
| def iterator: Iterator[A]^{this} = Iterator.empty ++ _reversed.iterator // very lazy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future head-scratchers: ++ on iterators are by-name on the RHS, and so will capture this by virtue of accessing the lazy val _reversed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to add check files for neg tests. That way we can also verify that the quality of the error messages does not regress.
|
|
||
| // Test case 1: Read-only access in initializer - should be OK | ||
| lazy val lazyVal: () ->{r.rd} Int = | ||
| val current = r2.get() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normal lazy vals should be able to capture exclusive capabilities. We are only restricting lazyvals in Mutable classes. These should behave like read-only methods. So the code in the compiler and the test here should be adapted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a commit that implements these suggestions.
|
It seems there's an issue with the following test program: class IO extends caps.SharedCapability:
def write(): Unit = ()
class C(val io: IO):
lazy val x = () => io.write()
val y: () ->{io} Unit = x
val c = C(io)
lazy val l1 = () => c.io.write()
lazy val l2 = () => this.io.write()
val s1 = () => c.io.write()
val s2 = () => this.io.write()
def test(io: IO) =
lazy val x = () => io.write()
val y: () ->{io} Unit = x
val c = C(io)
lazy val z = () => c.io.write()If I compile that with -Xprint:cc, I see: [[syntax trees at end of cc]] // lazyvals6.scala
package <empty> {
@SourceFile("lazyvals6.scala") class IO() extends Object(),
(scala.caps.SharedCapability^) {
def write(): Unit = ()
}
@SourceFile("lazyvals6.scala") class C(io: IO^) extends Object() {
val io: IO^
lazy val x: () ->{C.this.io} Unit = () => C.this.io.write()
val y: () ->{C.this.io} Unit = this.x
val c: C{val io: IO^{C.this.io}}^{C.this.io} = new C(C.this.io)
lazy val l1: () ->{} Unit = () => this.c.io.write()
lazy val l2: () ->{C.this.io} Unit = () => this.io.write()
val s1: () ->{} Unit = () => this.c.io.write()
val s2: () ->{C.this.io} Unit = () => this.io.write()
}
final lazy module val lazyvals6$package: lazyvals6$package =
new lazyvals6$package()
@SourceFile("lazyvals6.scala") final module class lazyvals6$package() extends
Object() {
private[this] type $this = lazyvals6$package.type
private def writeReplace(): AnyRef =
new scala.runtime.ModuleSerializationProxy(classOf[lazyvals6$package.type]
)
def test(io: IO^): Unit =
{
lazy val x: () ->{io} Unit = () => io.write()
val y: () ->{io} Unit = x
val c: C{val io: IO^{io}}^{io} = new C(io)
lazy val z: () ->{c.io} Unit = () => c.io.write()
()
}
}
}The types of |
|
I am actually not sure this has anything to do with lazyvals since the results are the same for |
a1b2ef6 to
f06d51c
Compare
|
I've added the requested changes, rebased, and squashed @odersky |
Add capture checking support for lazy vals
Fixes #21601
Extends capture checking to handle lazy vals, treating them like parameterless methods for capture tracking.
Key Changes
Lazy val environments.
Lazy vals now create their own environment to track initializer captures, separate from the result type's capture set.
Member selection.
When accessing a lazy val member (e.g.,
t.lazyMember), the qualifiertis charged to the capture set, reflecting that initialization may use the qualifier's capabilities.Read-only initialization.
For lazy val fields of
Mutableclasses, their initializers cannot call update methods on non-local exclusive capabilities (just like non-update methods).Documentation in the language reference.
Fix errors in the standard library revealed by proper lazy vals support.